[WRONG BRANCH] fix(config): omit webhook credentials from config export - #461
[WRONG BRANCH] fix(config): omit webhook credentials from config export#461luvs01 wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98818c105e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!path) throw new CliUsageError("export path is required", USAGE); | ||
| rejectArgs(args, USAGE); | ||
| const content = `${JSON.stringify(readConfigDiagnostics().config, null, 2)}\n`; | ||
| const content = `${JSON.stringify(omitWebhookCredentials(readConfigDiagnostics().config), null, 2)}\n`; |
There was a problem hiding this comment.
When <path> is the active config.json, this sanitized snapshot is written back over the live configuration, permanently deleting quotaResetNotify.webhookUrl and silently disabling webhook notifications after the next reload. Previously, exporting to that path preserved the full config; compare the resolved destination with getConfigPath() and reject an in-place sanitized export before writing.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
1 participant
Motivation
webhookUrlvalues when an operator runsocx config export, because the export path previously serializedreadConfigDiagnostics().configwithout applying the existing redaction boundary.Description
omitWebhookCredentialsand apply it to the export sink soocx config exportnever emits keys namedwebhookUrl(file:src/cli/config-command.ts).config showredaction behavior while ensuring both stdout and file export destinations are filtered before serialization (the export branch now callsomitWebhookCredentials(readConfigDiagnostics().config)).tests/usage/quota-reset-notify.test.tsto cover both stdout (export -) and file export cases and reuse a smallconfigureWebhookfixture helper.src/cli/config-command.tsandtests/usage/quota-reset-notify.test.tsto minimize surface area.Testing
bun test tests/usage/quota-reset-notify.test.ts, which passed (29 passing tests, 0 failures).bun run typecheckand the repository privacy scan withbun run privacy:scan, both of which succeeded.bun run test) was executed by CI locally but encountered unrelated environment-sensitive failures in other integration tests; the focused regression, typecheck, and privacy scan that validate this change passed.Codex Task